-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[camerax] Adds functionality to bind UseCases to a lifecycle #6939
Conversation
| @Override | ||
| public void onAttachedToActivity(@NonNull ActivityPluginBinding activityPluginBinding) { | ||
| updateContext(activityPluginBinding.getActivity()); | ||
| CameraAndroidCameraxPlugin plugin = new CameraAndroidCameraxPlugin(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I missed this in the last PR. But I don't think it is necessary to create a new CameraAndroidCameraxPlugin instance. You should be able to just call setUp on the current instance, right?
| @NonNull Long cameraSelectorIdentifier, | ||
| @NonNull List<Long> useCaseIds) { | ||
| ProcessCameraProvider processCameraProvider = | ||
| (ProcessCameraProvider) instanceManager.getInstance(identifier); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you wrap instanceManager.getInstance(..) with Objects.requireNonnull(...). It helps with debugging and removes the lint warning. Same for the line below and other apis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @gmackall just a heads up about this
...mera/camera_android_camerax/android/src/test/java/io/flutter/plugins/camerax/CameraTest.java
Outdated
Show resolved
Hide resolved
...mera/camera_android_camerax/android/src/test/java/io/flutter/plugins/camerax/CameraTest.java
Show resolved
Hide resolved
| /// the [ProcessCameraProvider] instance. | ||
| int getProcessCameraProviderIdentifier(ProcessCameraProvider instance) { | ||
| int? identifier = instanceManager.getIdentifier(instance); | ||
| identifier ??= instanceManager.addDartCreatedInstance(instance, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this add the instance to the InstanceManager if it doesn't contain the ProcessCameraProvider.?Shouldn't getInstancefromInstances add the ProcessCameraProvider to the InstanceManager? This should throw an error if the InstanceManager doesn't contain instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. Fixed that.
bparrishMines
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nits
...oid_camerax/android/src/main/java/io/flutter/plugins/camerax/CameraAndroidCameraxPlugin.java
Outdated
Show resolved
Hide resolved
...merax/android/src/main/java/io/flutter/plugins/camerax/ProcessCameraProviderHostApiImpl.java
Outdated
Show resolved
Hide resolved
…o/flutter/plugins/camerax/ProcessCameraProviderHostApiImpl.java Co-authored-by: Maurice Parrish <10687576+bparrishMines@users.noreply.github.com>
* bc0174fd5 [path_provider] Fix iOS `getApplicationSupportDirectory` regression (flutter/plugins#7026) * 15d799b89 [url_launcher] Convert Windows to Pigeon (flutter/plugins#6991) * dc8ad7701 Roll Flutter from c35efda to a815ee6 (22 revisions) (flutter/plugins#7025) * e9406bc20 [camerax] Adds functionality to bind UseCases to a lifecycle (flutter/plugins#6939)
Adds
UseCaseandCameraclasses and adds methods in theProcessCameraProviderclass to allow for the binding ofUseCases to the lifecycle of aCamera.Fixes flutter/flutter#111220.
Part of flutter/flutter#111127, flutter/flutter#115846.
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.